PYTHON-5536 Avoid clearing the connection pool when the server connection rate limiter triggers by blink1073 · Pull Request #2509 · mongodb/mongo-python-driver

blink1073 · 2025-08-26T01:51:32Z

Currently testing with this script for async:

import asyncio
from pymongo import AsyncMongoClient

client = AsyncMongoClient(maxConnecting=100)

async def target():
    await client.admin.command("ping")

async def main():
    await client.admin.command('setParameter', 1, ingressConnectionEstablishmentRateLimiterEnabled=True)
    await client.admin.command('setParameter', 1, ingressConnectionEstablishmentRatePerSec=30)
    await client.admin.command('setParameter', 1, ingressConnectionEstablishmentBurstCapacitySecs=1)
    await client.admin.command('setParameter', 1, ingressConnectionEstablishmentMaxQueueDepth=1)

    # Warm the pool so there are existing connections.
    tasks = []
    for i in range(10):
        tasks.append(asyncio.create_task(target()))
    await asyncio.wait(tasks)

    tasks = []
    for i in range(200):
        tasks.append(asyncio.create_task(target()))

    await asyncio.wait(tasks)


asyncio.run(main())

and this one for sync:

from pymongo import MongoClient
from concurrent.futures import ThreadPoolExecutor, wait

client = MongoClient(maxConnecting=100)

def target():
    client.admin.command("ping")

def main():
    client.admin.command('setParameter', 1, ingressConnectionEstablishmentRateLimiterEnabled=True)
    client.admin.command('setParameter', 1, ingressConnectionEstablishmentRatePerSec=30)
    client.admin.command('setParameter', 1, ingressConnectionEstablishmentBurstCapacitySecs=1)
    client.admin.command('setParameter', 1, ingressConnectionEstablishmentMaxQueueDepth=1)

    # Warm the pool so there are existing connections.
    print("1")
    tasks = []
    pool = ThreadPoolExecutor(200)
    for i in range(10):
        tasks.append(pool.submit(target))
    wait(tasks)
    print("2")
    tasks = []
    for i in range(200):
        tasks.append(pool.submit(target))
    wait(tasks)

main()

…mongodb#2498)

…#2499)

…ongodb#2503)

)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…b#2507) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…ction rate limiter triggers

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Steven Silvester <steve.silvester@mongodb.com>

ShaneHarvey · 2025-08-26T16:51:37Z

pymongo/asynchronous/pool.py

+                            conn.conn.get_conn.read(1)
+                        except Exception as _:
+                            # TODO: verify the exception
+                            close_conn = False


2 comments:

I believe this logic needs to move to connection checkout. Here in connection check in we already know the connection is useable because we're checking it back in after a successful command.

Instead of a 1ms read can we reuse the existing _perished() + conn_closed() methods?

ShaneHarvey

Nice work!

…DRIVERS-3218

…er into DRIVERS-3218" This reverts commit c1fe2e3, reversing changes made to f51e8a5.

(cherry picked from commit 0d4c84e)

blink1073 · 2025-09-11T11:45:13Z

Ideally we'd like to detect connection reset by peer errors and only backoff for those, but in my testing, during backoff, we can sometimes get a regular EOF instead of a closing error, which triggers the raise OSError("connection closed") error.

NoahStapp · 2025-09-11T14:47:45Z

pymongo/network_layer.py

        else:
-            if self._closing_exception:
-                raise self._closing_exception
+            if self._closed.done():


Is calling is_closing here better? It'll catch more edge cases in theory.

Hmm let me try that.

No, it is ambiguous as to whether connection_lost as been called yet. Since connection_lost is synchronous, checking for self._closed.done() assures that we have actually lost the connection.

ShaneHarvey · 2025-09-19T18:03:48Z

pymongo/asynchronous/pool.py

+        ):
+            self._backoff += 1
+            error._add_error_label("SystemOverloaded")
+            error._add_error_label("Retryable")


Could you merge backpressure? Originally I added the incorrect labels here. It should be "SystemOverloadedError" and "RetryableError"

ShaneHarvey · 2025-09-19T18:03:51Z

pymongo/asynchronous/pool.py

+            self._backoff += 1
+            error._add_error_label("SystemOverloaded")
+            error._add_error_label("Retryable")
+            print(f"Setting backoff in {phase}:", self._backoff)  # noqa: T201


Instead of inspecting the error message after the fact, is it possible we can record some state to determine if the error happened during DNS+TCP or after? Like:

# Assume all non dns/tcp/timeout errors mean the server rejected the connection due to overload. if not errorDuringDnsTcp and not timeoutError: error._add_error_label("SystemOverloadedError")

DNS is already resolved by the time we make a pool as far as I can tell, and we can't distinguish between TCP connection and TLS handshake for async.

I added some logic in 7548f7b that looks for a specific error attached to AutoReconnect.

For discussion: we can still run into the condition were we hit this line, and there is no closing error, and we have not yet received any data on the protocol. I verified this by setting a flag when buffer_updated is called. We don't have a way to ascribe more semantic meaning to this condition as far as I can tell from the Protocol/Transport docs and the available properties on each.

The problem got even worse when using gevent, since the error was something completely different, so I reverted any extra handling of the connection error.

After today's discussion I added 6a47fc6 to only add the labels if we have completed DNS+TCP connection.

… into DRIVERS-3218

blink1073 · 2025-09-22T22:23:13Z

Successful PERF run with the current state (f1294dc): https://spruce.mongodb.com/task/sys_perf_perf_3_node_replSet.availability.arm.aws.2024_05_connection_storm_rate_limited_locust_patch_d67246eb210c68496ec38c597f9a1e2cb76af448_68d1c72d8f08a5000737aaf6_25_09_22_22_01_34/logs?execution=0

ShaneHarvey · 2025-10-08T19:07:23Z

pymongo/asynchronous/pool.py

+        self._backoff += 1
+        error._add_error_label("SystemOverloadedError")
+        error._add_error_label("RetryableError")
+        print(f"Setting backoff in {phase}:", self._backoff)  # noqa: T201


Could we remove this print now?

Do we want to log it somehow? In the Server Tech Design they call out "A message will be logged and the function will be returned from."

Yes we'll add a CMAP log message and/or an a new event. Feel free to add a log message now or we can defer to the spec PR.

ShaneHarvey · 2025-10-08T19:11:57Z

pymongo/synchronous/pool.py

+                self._handle_connection_error(error, "handshake")
            if isinstance(error, (IOError, OSError, *SSLErrors)):
                details = _get_timeout_details(self.opts)
                _raise_connection_failure(self.address, error, timeout_details=details)


_raise_connection_failure will raise a new exception in which case the error labels won't be propagated. Is that a bug? I guess it's not because _raise_connection_failure only handles IOError, OSError, *SSLErrors while _handle_connection_error only handles AutoReconnect, right?

Correct, they are orthogonal

ShaneHarvey · 2025-10-08T19:13:34Z

Looks like there's still a few load balancer spec tests that fail due to the new behavior: https://spruce.mongodb.com/task/mongo_python_driver_backpressure_load_balancer_test_non_standard_latest_python3.14_auth_ssl_sharded_cluster_patch_c458379522a3ad0ff0cb577aea1f9fe22efd1bfe_68d32dc8fea2ae0007bdadfb_25_09_23_23_31_22/tests?execution=0&sorts=STATUS%3AASC

ShaneHarvey

LGTM!

…tion rate limiter triggers (#2509) Co-authored-by: Iris <58442094+sleepyStick@users.noreply.github.com> Co-authored-by: Noah Stapp <noah.stapp@mongodb.com> Co-authored-by: Shane Harvey <shnhrv@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> PYTHON-5629 Increase max overload retries from 3 to 5 and initial delay from 50ms to 100ms (#2599) PYTHON-5517 Simplify pool backpressure behavior (#2611) synchro update network_layer update pool shared update pool shared update run-tests

…tion rate limiter triggers (mongodb#2509) Co-authored-by: Iris <58442094+sleepyStick@users.noreply.github.com> Co-authored-by: Noah Stapp <noah.stapp@mongodb.com> Co-authored-by: Shane Harvey <shnhrv@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…r connection rate limiter triggers (mongodb#2509)" This reverts commit a99645e.

blink1073 and others added 15 commits August 19, 2025 11:23

PYTHON-5503 Use uv to install just in GitHub Actions (mongodb#2490)

d24b4a5

PYTHON-5502 Fix c extensions on OIDC VMs (mongodb#2489)

3a26119

Prep for 4.14.1 release (mongodb#2495) [master] (mongodb#2496)

db3d3c7

PYTHON-5143 Support auto encryption in unified tests (mongodb#2488)

f7b94be

PYTHON-5496 Update CSOT tests for change in dropIndex behavior in 8.3 (…

9a9a65c

…mongodb#2498)

PYTHON-5508 - Add built-in DecimalEncoder and DecimalDecoder (mongodb…

5e96353

…#2499)

PYTHON-5456 Support text indexes with auto encryption (mongodb#2500)

e08284b

PYTHON-5510 Fix server selection log message for commitTransaction (m…

ddf9508

…ongodb#2503)

PYTHON-5514 Specific assertions for "is" and "is not None" (mongodb#2502

3ebd934

)

Bump pyright from 1.1.403 to 1.1.404 (mongodb#2506)

cd4e5db

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Update coverage requirement from <=7.10.3,>=5 to >=5,<=7.10.5 (mongod…

9892e1b

…b#2507) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

DRIVERS-3218 Avoid clearing the connection pool when the server conne…

1179c5c

…ction rate limiter triggers

set to one byte

bc91967

Bump the actions group with 5 updates (mongodb#2505)

8c361be

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Steven Silvester <steve.silvester@mongodb.com>

PYTHON-5519 Clean up uv handling (mongodb#2510)

0d4c84e

ShaneHarvey reviewed Aug 26, 2025

View reviewed changes

blink1073 and others added 13 commits August 26, 2025 19:45

update approach

f51e8a5

Merge branch 'master' of github.com:mongodb/mongo-python-driver into …

c1fe2e3

…DRIVERS-3218

Revert "Merge branch 'master' of github.com:mongodb/mongo-python-driv…

7584d2d

…er into DRIVERS-3218" This reverts commit c1fe2e3, reversing changes made to f51e8a5.

undo topology changes

f1544aa

improve sleep translation

9d34e52

improve sleep translation

bb5ac35

PYTHON-5519 Clean up uv handling (mongodb#2510)

957a87d

(cherry picked from commit 0d4c84e)

add prose tests

9d0af17

debug

da0c0e5

fix and update tests

70b4113

fix logic

c974d36

only backoff if conn is closed

845f17a

use AutoReconnect

09fc66d

blink1073 added 2 commits September 11, 2025 09:20

handle the already closed case

a033c58

handle another edge case

ded90b0

NoahStapp reviewed Sep 11, 2025

View reviewed changes

ShaneHarvey reviewed Sep 19, 2025

View reviewed changes

blink1073 added 5 commits September 22, 2025 07:28

Merge branch 'backpressure' of github.com:mongodb/mongo-python-driver…

2cd3c18

… into DRIVERS-3218

address review

0b4b265

clean up logic

7548f7b

clean up logic

5e34bdc

do not try and label errors

f1294dc

blink1073 added 2 commits September 23, 2025 17:13

address review

6a47fc6

fix option pass

6d57549

ShaneHarvey reviewed Oct 8, 2025

View reviewed changes

blink1073 added 7 commits October 8, 2025 16:04

update load balancer sdam tests

9fa3a6f

update uri options

47e55b4

create new client

4f06bb6

try again

3c760e7

address review

4cc6619

add pool backoff message

eaab484

update enum

65832a7

ShaneHarvey approved these changes Oct 9, 2025

View reviewed changes

blink1073 merged commit d267eb4 into mongodb:backpressure Oct 9, 2025
77 of 79 checks passed

blink1073 deleted the DRIVERS-3218 branch October 9, 2025 16:43

This was referenced Nov 10, 2025

Revert "PYTHON-5536 Avoid clearing the connection pool when the server connection rate limiter triggers" #2610

Closed

PYTHON-5517 Simplify pool backpressure behavior #2611

Merged

sleepyStick added a commit to sleepyStick/mongo-python-driver that referenced this pull request Nov 25, 2025

Revert "PYTHON-5536 Avoid clearing the connection pool when the serve…

40d8ff6

…r connection rate limiter triggers (mongodb#2509)" This reverts commit a99645e.

Conversation

blink1073 commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShaneHarvey Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey left a comment

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Sep 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey commented Oct 8, 2025

Uh oh!

ShaneHarvey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

blink1073 commented Aug 26, 2025 •

edited

Loading

ShaneHarvey Aug 26, 2025 •

edited

Loading

blink1073 commented Sep 22, 2025 •

edited

Loading